Skip to content

Use principal to classify system workers#10143

Merged
rkannan82 merged 29 commits intomainfrom
kannan/system-worker-principal
May 4, 2026
Merged

Use principal to classify system workers#10143
rkannan82 merged 29 commits intomainfrom
kannan/system-worker-principal

Conversation

@rkannan82
Copy link
Copy Markdown
Contributor

@rkannan82 rkannan82 commented May 1, 2026

What

Use the gRPC principal to classify workers as system workers at upsert time, falling back to task queue prefix matching when principal is not available.

Why

Principal-based classification is more robust than string matching on task queue names and works correctly when principal propagation is enabled.

How did you test it?

Unit tests covering three scenarios:

  • temporal principal marks worker as system
  • non-temporal principal overrides task queue prefix
  • nil principal falls back to prefix matching

🤖 Generated with Claude Code

rkannan82 and others added 13 commits April 29, 2026 15:53
System workers (on temporal-sys-* task queues) are now filtered out of
ListWorkers responses unless the caller sets IncludeSystemWorkers=true.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers both "temporal-sys-*" (e.g. temporal-sys-per-ns-tq) and
"/temporal-sys/*" (e.g. /temporal-sys/worker-commands/...) prefixes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verifies that page sizes and tokens operate on the filtered set,
not the full set including system workers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…me at query time

Moves the system worker classification to the entry level so it's computed
once during heartbeat recording rather than on every ListWorkers call.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename local variable for clarity
- Add test for filterWorkers with includeSystemWorkers=false
- Annotate bool args with /*includeSystemWorkers*/ for readability

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Note: this introduces a build failure in client/frontend due to the new
PauseActivityExecution RPC added in temporalio/api#743. That method needs
stubs before the full repo can build.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a principal is available from the gRPC context, use its type to
determine if a worker is a system worker (type="temporal"). When
principal is not available (propagation disabled), fall back to the
existing task queue prefix check.

Also fixes isSystemWorkerWorker typo from previous rename.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rkannan82 rkannan82 force-pushed the kannan/system-worker-principal branch from 748ec5a to 1ae9010 Compare May 1, 2026 03:10
Require type="temporal" AND name="internal" to match the exact identity
produced by internalClaimMapper. Also add test for temporal type with
non-internal name.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rkannan82 rkannan82 marked this pull request as ready for review May 1, 2026 03:17
@rkannan82 rkannan82 requested a review from a team as a code owner May 1, 2026 03:17
@rkannan82 rkannan82 requested a review from simvlad May 1, 2026 03:18
Comment thread service/matching/workers/registry_impl.go Outdated
rkannan82 and others added 2 commits May 1, 2026 17:19
…xclude-system

# Conflicts:
#	common/primitives/task_queues.go
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Base automatically changed from kannan/list-workers-exclude-system to main May 2, 2026 00:49
@rkannan82 rkannan82 requested review from a team as code owners May 2, 2026 00:49
rkannan82 and others added 3 commits May 1, 2026 19:57
…principal

# Conflicts:
#	service/matching/workers/registry_impl.go
#	service/matching/workers/registry_impl_test.go
#	service/matching/workers/registry_test.go
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rkannan82 and others added 3 commits May 1, 2026 20:09
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rkannan82 rkannan82 enabled auto-merge (squash) May 2, 2026 03:13
rkannan82 and others added 2 commits May 1, 2026 20:20
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rkannan82 rkannan82 requested a review from ShahabT May 2, 2026 15:58
rkannan82 and others added 3 commits May 3, 2026 15:32
Replace string literals with constants from common/authorization/principal.go
(merged in #10163).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@ShahabT ShahabT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm otherwise

// If a principal is available, it checks whether the principal identifies
// the Temporal server itself (type="temporal", name="internal"). Otherwise,
// it falls back to checking the task queue name prefix.
func isSystemWorker(principal *commonpb.Principal, taskQueue string) bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a global helper?

also, is it possible to have principal.GetType() == authorization.InternalPrincipalType but principal.GetName() != authorization.InternalPrincipalName ? (I wonder if we should only check the first one)

@rkannan82 rkannan82 merged commit 18d5ae9 into main May 4, 2026
47 checks passed
@rkannan82 rkannan82 deleted the kannan/system-worker-principal branch May 4, 2026 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants